Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split ConsoleKit into two functional targets #192

Merged
merged 6 commits into from
Dec 1, 2023
Merged

Split ConsoleKit into two functional targets #192

merged 6 commits into from
Dec 1, 2023

Conversation

gwynne
Copy link
Member

@gwynne gwynne commented Nov 13, 2023

ConsoleKit has been split into two separate targets:

  • ConsoleKitTerminal contains the logic for interacting with a console (I/O, color support, other ANSI commands, etc.).
  • ConsoleKitCommands contains the functionality for handling argument parsing, which is now soft-deprecated in favor of swift-argument-parser when possible.

The existing ConsoleKit target is now an umbrella import, similar to the function of the NIO target in swift-nio; existing code should be unaffected.

Several additional minor cleanups were also made in the process of this split.

@gwynne gwynne added the semver-minor When merged, a new minor version release will be generated label Nov 13, 2023
Sources/ConsoleKitCommands/Async/AsyncCommandGroup.swift Outdated Show resolved Hide resolved
Sources/ConsoleKitCommands/CommandGroup.swift Outdated Show resolved Hide resolved
Sources/ConsoleKitCommands/Command.swift Outdated Show resolved Hide resolved
Sources/ConsoleKitCommands/Utilities.swift Outdated Show resolved Hide resolved
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation needs updating otherwise mainly LGTM

@@ -16,7 +16,6 @@ struct AsyncExample {
try await console.run(group, input: input)
} catch let error {
console.error("\(error)")
exit(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not exit(1) anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'cause it had no meaningful effect, and exiting with a nonzero status is arguably a questionable behavior to provide as an example (especially when exit(1) isn't a fully platform-agnostic or even necessarily correct means of doing so)

Sources/ConsoleKitCommands/Docs.docc/index.md Outdated Show resolved Hide resolved
Sources/ConsoleKitCommands/Docs.docc/index.md Outdated Show resolved Hide resolved
Sources/ConsoleKitTerminal/Docs.docc/index.md Outdated Show resolved Hide resolved
Sources/ConsoleKitTerminal/Docs.docc/index.md Outdated Show resolved Hide resolved
Copy link
Member

@ptoffy ptoffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to add to the other reviews

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

Merging #192 (b1301dd) into main (2e3e205) will decrease coverage by 0.68%.
The diff coverage is 58.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   49.28%   48.61%   -0.68%     
==========================================
  Files          45       44       -1     
  Lines        1901     1874      -27     
==========================================
- Hits          937      911      -26     
+ Misses        964      963       -1     
Files Coverage Δ
...s/ConsoleKitCommands/Async/AsyncCommandGroup.swift 90.74% <100.00%> (ø)
...urces/ConsoleKitCommands/Async/AsyncCommands.swift 0.00% <ø> (ø)
...mands/Async/GenerateAsyncAutocompleteCommand.swift 0.00% <ø> (ø)
...urces/ConsoleKitCommands/Base/CommandContext.swift 100.00% <ø> (ø)
Sources/ConsoleKitCommands/Base/CommandGroup.swift 90.74% <100.00%> (ø)
Sources/ConsoleKitCommands/Base/CommandInput.swift 97.82% <ø> (ø)
Sources/ConsoleKitCommands/Base/Commands.swift 0.00% <ø> (ø)
Sources/ConsoleKitCommands/Base/Console+Run.swift 86.66% <ø> (ø)
...mands/Completion/GenerateAutocompleteCommand.swift 0.00% <ø> (ø)
...urces/ConsoleKitCommands/Signatures/Argument.swift 80.95% <ø> (ø)
... and 34 more

gwynne and others added 6 commits December 1, 2023 10:47
…action, one for command handling, plus an umbrella for compatibility. Also update README, and docs images. Clean up several bits of code.
Co-authored-by: Mahdi Bahrami <github@mahdibm.com>
Co-authored-by: Tim Condon <0xTim@users.noreply.github.com>
…sons learned from apple/swift-argument-parser#590, pretty much just for the hell of it. Deprecate `ConsoleErrror`.
@gwynne gwynne enabled auto-merge (squash) December 1, 2023 17:01
@gwynne gwynne merged commit 0bd6673 into main Dec 1, 2023
12 of 13 checks passed
@gwynne gwynne deleted the split-targets branch December 1, 2023 17:03
@penny-for-vapor
Copy link

These changes are now available in 4.12.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor When merged, a new minor version release will be generated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants